Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track users when joining encrypted rooms #251

Merged
merged 3 commits into from
Sep 21, 2022

Conversation

B4dM4n
Copy link
Contributor

@B4dM4n B4dM4n commented Aug 1, 2022

Users are only tracked when the bot user is already in the room when encryption is enabled or if the member status of the user changes in an already encrypted room.
This leads to megolm sessions which don't include all devices when a bot joins an encrypted room and existing users can't decrypt messages by the bot.

Before this change, only in rooms created by the bot could all other users decrypt messages from the bot.
Now already present users will also be tracked and receive the session keys.

A simple appservice reproducing the error, which is based on the encryption_appservice.ts example, can be found here (also contains docker-compose file for synapse and co):
https://github.com/B4dM4n/hookkshot-e2ee-test/blob/main/enctest/src/index.ts

Steps to reproduce:

  • Invite @encbot:local to encrypted DM/room (inviting user is not tracked by the bot)
  • Send !ping to bot -> unable to decrypt error
  • Send !invite to bot and join room (invited user is now tracked)
  • Send !ping to room -> Pong can be decrypted

Checklist

  • Tests written for all new code
  • Linter has been satisfied - No idea how to check
  • Sign-off given on the changes (see CONTRIBUTING.md)

Users are only tracked when the bot user is already in the room when
encryption is enabled or if the member status of the user changes in an
already encrypted room.
This leads to megolm sessions which don't include all devices when a bot
joins an encrypted room and existing users can't decrypt messages by the
bot.
Before this change, only in rooms created by the bot could all other
users decrypt messages from the bot.
Now already present users will also be tracked and receive the session
keys.

Signed-off-by: Fabian Möller <fabianm88@gmail.com>
Copy link
Owner

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good, though if I can get tests added then that would be most appreciated :)

@B4dM4n
Copy link
Contributor Author

B4dM4n commented Aug 8, 2022

While working on the tests for this, I noticed another error related to user tracking: #254

I will update this PR once the other issue is fixed.

Copy link
Owner

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I've adjusted the logic slightly to avoid over-tracking users, but generally this seems sane to me.

@turt2live turt2live enabled auto-merge (squash) September 21, 2022 20:14
@turt2live turt2live merged commit 9e744cb into turt2live:main Sep 21, 2022
@B4dM4n B4dM4n deleted the e2ee-track-users branch September 23, 2022 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants